Skip to content

Comments

Local task overrides#26

Open
ernestvh wants to merge 2 commits intoFlorentRevest:mainfrom
ernestvh:local-task-overrides
Open

Local task overrides#26
ernestvh wants to merge 2 commits intoFlorentRevest:mainfrom
ernestvh:local-task-overrides

Conversation

@ernestvh
Copy link

Hi,

Thanks a lot for your work here, this repository has given my workflow a large QoL upgrade.

For my workflow, I wanted a different defconfig task (we merge kconfig fragments to generate an initial .config instead of calling make defconfig) and the only way to accomplish my behaviour was quite hacky, so instead I added these commits to allow redefining a task in local.sh. I believe that can be useful for others as well.

The default behaviour without any customization of local.sh should remain unchanged.

Kind regards,
Ernest

Ernest Van Hoecke added 2 commits February 20, 2026 15:31
This allows using $MAKE_VARS elsewhere even without invoking make
directly. For example, to call scripts/kconfig/merge_config.sh from a
task and passing it the same environment variables.

The existing behaviour is unaffected.

Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
Adds a local task override hook in tasks.sh that calls a function in
local.sh and exits before the default tasks.sh task is run.

This hook runs after all variables are defined, while local.sh has to be
sourced before it to allow it to set defaults. The hook thus allows
users to redefine commands in local.sh that use variables such as $MAKE
and in the normal context.

Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
## Enable some random kernel CONFIG by default as part of the .config generation
# if [ $COMMAND = "defconfig" ]; then
# trap "scripts/config -e BPF_SYSCALL" EXIT
# fi
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you considered doing something like this block with an exit at the end to replace commands instead ?

This is how I originally envisioned this kind of override to work. It's not suupeer elegant of course but it keeps the complexity all contained in local.sh which is what I've generally tried to do

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trouble is that this then applies on top of the original command.

For example, defconfig runs only if .config does not exist yet. So my custom defconfig task would not run if it has the same guard. Of course I could work around that, I could first in local.sh, without an exit trap, check if .config exists and then if it does not, touch it, and then in the exit trap create it with my own logic. That is also acceptable to me if such hacks are more aligned with how you'd like to structure this project.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry the example I picked cause more confusion than I thought.

I didn't mean to point at the trap "scripts/config -e BPF_SYSCALL" EXIT line but rather at the condition itself.

Essentially I was suggesting something like:

if [ $COMMAND = "defconfig" ]; then
  # here your custom logic, can be guarded by a check for .config if you'd like
  exit 0 # this is the exit I was talking about, that will prevent the "defconfig" command defined in tasks.sh from running
fi

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes this is what I initially wanted to do, but then your logic runs before many variables are set in tasks.sh. Using make for example will not work since the $MAKE variable is not defined yet. You can of course define a default for it yourself in local.sh, but you might end up duplicating parts of tasks.sh there.

Maybe an alternative is moving the sourcing of local.sh to after the setting of default variables. In that case local.sh would override the existing variables if desired, instead of defining them first. This ties into the discussion here: #12, where it seems like you had a good reason not to do so.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe an alternative is moving the sourcing of local.sh to after the setting of default variables.

Or keeping that there, but allow sourcing other files after having set the default variables?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right - that makes sense, thanks. :)

My current line of thinking is that we could make all that override logic less "special cased" if we'd refactor things such that:

  • tasks.sh defines quite early on functions like: task_start, task_start_wait_dbg, task_stop etc...
  • local.sh is free to override these functions if it wants to, or to wrap them, eg:
# in tasks.sh
task_defconfig() {
    echo "shared defconfig"
}
# in local.sh
eval "$(declare -f task_defconfig | sed '1s/task_defconfig/old_task_defconfig/')"
task_defconfig() {
    echo "adding things before..."
    old_task_defconfig
    echo "adding things after..."
}

Then we could also get rid of the trap "scripts/config -e BPF_SYSCALL" EXIT hack and the overriding logic would feel generally more consistent and explicit.

Essentially this would take your idea but one step further. Does that make sense ? Sorry I'm using you for rubber ducking :)


The env var override part is another part I've not been very happy with but this also makes knots between my neurons so let's keep this separate for now. But essentially I've been thinking that if those environment variables had also been functions instead, then we wouldn't have to think so hard about the order in which they are declared or overriden because right now if you override TARGET_ARCH too late then other variables like IMAGE_PATH would still refer to the old arch... But if local.sh would override an arch() function instead, then an image_path() function would magically work, but users would still be free to override image_path() if they'd like. I'm trying to optimize for readability and ease of modification so my fear is that it would make the script more scary to users.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like this idea. If they're all declared as functions upfront and then called later, it improves the readability of the script and the extensibility in local.sh, without increasing the complexity (in my brain, at least). Rubber duck ahead, I'm glad to improve my tooling :)


This is interesting, but I agree that it might make the script scarier. I could see myself even simply sourcing some .env in my local.sh later, instead of cleanly setting variables that are passed around, just so I can also source the exact same thing in a terminal and understand both workflows easily. Going via functions might add a confusing layer of indirection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants